Skip to content

Fix history events buildup from version reject#1220

Merged
cgillum merged 2 commits into
Azure:mainfrom
halspang:halspang/fix_reject_history
May 23, 2025
Merged

Fix history events buildup from version reject#1220
cgillum merged 2 commits into
Azure:mainfrom
halspang:halspang/fix_reject_history

Conversation

@halspang

Copy link
Copy Markdown
Member

When an orchestration was being rejected due to a versioning mismatch an issue would cause the history events of the orchestration to build continuously. This was caused by the orchestration adding an OrchestrationStarted event before the versioning took place and was saved even on a rejected version.

This change causes the versioning code to exit the process method entirely instead of just prematurely end the process loop. This avoids the commit of the history.

@halspang halspang requested review from AnatoliB and cgillum May 22, 2025 21:11
@AnatoliB

Copy link
Copy Markdown
Contributor

Do you think we have sufficient test coverage for this? Wondering if it would be possible (and practical) to add a test that fails without this change.

When an orchestration was being rejected due to a versioning mismatch
an issue would cause the history events of the orchestration to build
continuously. This was caused by the orchestration adding an
OrchestrationStarted event before the versioning took place and was
saved even on a rejected version.

This change causes the versioning code to exit the process method
entirely instead of just prematurely end the process loop. This avoids
the commit of the history.

Signed-off-by: halspang <halspang@microsoft.com>
@halspang halspang force-pushed the halspang/fix_reject_history branch from e550739 to e587e35 Compare May 23, 2025 00:02
@halspang

Copy link
Copy Markdown
Member Author

@AnatoliB - Updated a test that looks at this scenario. I confirmed manually that it passes with this change and fails without it.

@cgillum cgillum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the fix is 90% correct. See my one request below about changing the false value to true. Then I'll be ready to sign off.

Comment thread src/DurableTask.Core/TaskOrchestrationDispatcher.cs Outdated
Signed-off-by: halspang <halspang@microsoft.com>
@halspang halspang requested a review from cgillum May 23, 2025 21:25
@cgillum cgillum merged commit 6674c79 into Azure:main May 23, 2025
44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants